Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add --json flag for resource-class cmds #1088

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Add --json flag for resource-class cmds #1088

wants to merge 3 commits into from

Conversation

jdelnano
Copy link

@jdelnano jdelnano commented Dec 20, 2024 •
edited
Loading

Relates to #380

Checklist

=========

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have checked for similar issues and haven't found anything relevant.
  • This is not a security issue (which should be reported here: https://circleci.com/security/)
  • I have read Contribution Guidelines.

Internal Checklist

Not applicable to me, a non-CircleCI employee

Changes

=======

Implementation

I implemented my changes in the following way:

  • I added a local --json flag to the resource_class.go file where it was appropriate to, for the create and list command actions.
  • In each command, I check whether or not the --json flag was set on the command line, and if so, we Marshal the appropriate object and set it in the Writer interface offered by the cobra cmd object.
  • I introduced the JSON formatting logic such that the original CLI behavior is unchanged. If you submit commands like:
    • circleci runner resource-class list <>
    • circleci runner resource-class create <>

they will behave exactly as they did. Only if you include a --json flag will you see new behavior, returned to stdout.

Automated Tests

I made sure to add tests in resource_class_test.go where appropriate as well as update the expected usage files.

Manual Tests

I executed the following commands manually (alongside the automated testing), to ensure:

  • They work as expected, adding in the new JSON formatting.
  • The old CLI behavior is maintained as before these changes.
➜ make build  # the CLI builds successfully


➜ ./build/darwin/arm64/circleci runner resource-class list jdelnano --json

[{"id":"<redacted>","resource_class":"jdelnano/test","description":"test"}]

========================================================================================================================================================

➜ ./build/darwin/arm64/circleci runner resource-class list jdelnano
+-------------------------------------------------------------+--------------------------------------------------+
|                       RESOURCE CLASS                        |                   DESCRIPTION                    |
+-------------------------------------------------------------+--------------------------------------------------+
| jdelnano/test                                               | test                                             |
+-------------------------------------------------------------+--------------------------------------------------+

➜ ./build/darwin/arm64/circleci runner resource-class create jdelnano/test "test"
If you have not already agreed to Runner Terms in a signed Order, then by continuing to install Runner, you are agreeing to CircleCI's Runner Terms which are found at: https://circleci.com/legal/runner-terms/.
If you already agreed to Runner Terms in a signed Order, the Runner Terms in the signed Order supersede the Runner Terms in the web address above.
If you did not already agree to Runner Terms through a signed Order and do not agree to the Runner Terms in the web address above, please do not install or use Runner.

+-------------------------------------------+-------------+
|              RESOURCE CLASS               | DESCRIPTION |
+-------------------------------------------+-------------+
| jdelnano/test                             | test        |
+-------------------------------------------+-------------+

========================================================================================================================================================

➜ ./build/darwin/arm64/circleci runner resource-class create jdelnano/test "test" --generate-token
If you have not already agreed to Runner Terms in a signed Order, then by continuing to install Runner, you are agreeing to CircleCI's Runner Terms which are found at: https://circleci.com/legal/runner-terms/.
If you already agreed to Runner Terms in a signed Order, the Runner Terms in the signed Order supersede the Runner Terms in the web address above.
If you did not already agree to Runner Terms through a signed Order and do not agree to the Runner Terms in the web address above, please do not install or use Runner.

api:
    auth_token: <redacted>
+-------------------------------------------+-------------+
|              RESOURCE CLASS               | DESCRIPTION |
+-------------------------------------------+-------------+
| jdelnano/test                             | test        |
+-------------------------------------------+-------------+

========================================================================================================================================================

➜ ./build/darwin/arm64/circleci runner resource-class create jdelnano/test "test" --json
If you have not already agreed to Runner Terms in a signed Order, then by continuing to install Runner, you are agreeing to CircleCI's Runner Terms which are found at: https://circleci.com/legal/runner-terms/.
If you already agreed to Runner Terms in a signed Order, the Runner Terms in the signed Order supersede the Runner Terms in the web address above.
If you did not already agree to Runner Terms through a signed Order and do not agree to the Runner Terms in the web address above, please do not install or use Runner.

{"id":"<redacted>","resource_class":"jdelnano/test","description":"test"}

========================================================================================================================================================

➜ ./build/darwin/arm64/circleci runner resource-class create jdelnano/test "test" --generate-token --json
If you have not already agreed to Runner Terms in a signed Order, then by continuing to install Runner, you are agreeing to CircleCI's Runner Terms which are found at: https://circleci.com/legal/runner-terms/.
If you already agreed to Runner Terms in a signed Order, the Runner Terms in the signed Order supersede the Runner Terms in the web address above.
If you did not already agree to Runner Terms through a signed Order and do not agree to the Runner Terms in the web address above, please do not install or use Runner.

{"id":"<redacted>","token":"<redacted>","resource_class":"jdelnano/test","nickname":"default","created_at":"2024-12-27T18:43:48.345011Z"}

Rationale

=========

Background

In my position at Okta, a CircleCI customer, I have a few automation tasks that require the use of the circleci runner resource-class list and circleci runner resource-class create commands, where I specifically need to grab the output of these commands and do subsequent actions. As a result, my motivation for adding the --json flag support for the circleci runner resource-class command is to return more easily parsable output, instead of the default ASCII table format.

Potential Future Work

If this PR is accepted, and it is deemed advantageous, I can move this new --json flag to be global/applicable so that other CLI commands can return JSON formatted responses to stdout.

@jdelnano jdelnano requested a review from a team as a code owner December 20, 2024 20:18
@jdelnano jdelnano force-pushed the add-json-output-flag-for-resource-class-cmds branch from 4b4fb83 to 5ecaaae Compare December 20, 2024 20:25
@jdelnano jdelnano marked this pull request as draft December 20, 2024 20:26
@jdelnano jdelnano marked this pull request as ready for review December 27, 2024 18:39
@jdelnano
Copy link
Author

Open question:

Screenshot 2024-12-27 at 10 39 36 AM

I'm not sure why my build is being marked as Failed here, since I have all checks passing. Is it because I'm 'unauthorized' and cannot run the vulnerability scan as a non-CircleCI employee?

@adamharvey-okta
Copy link

@jdelnano Yeah, it's an unauth to a context. That's a set of secrets which may be being used by CCI to access their vuln vendor, and they would have assigned a user or groups to that context which you aren't in. Not 100% sure, but does infer open source contributors can't fully run PRs ourselves. I'm sure our CCI friends will take a peek!

@adamharvey-okta
Copy link

Ref:

circleci-cli/.circleci/config.yml

Lines 359 to 360 in 08776cb

- vulnerability-scan:
context: org-global-employees

@meeech
Copy link

meeech commented Jan 8, 2025

thanks @jdelnano for this - will give it a review

thanks @adamharvey-okta for helping out here as well to give @jdelnano that context.

christian-stephen
christian-stephen approved these changes Jan 8, 2025
View reviewed changes
Copy link
Contributor

@christian-stephen christian-stephen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you!

@christian-stephen christian-stephen requested a review from a team January 8, 2025 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@christian-stephen christian-stephen christian-stephen approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jdelnano @adamharvey-okta @meeech @christian-stephen